Skip to content

feat: save table filters in local storage #1143

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Jul 16, 2025
Merged

Conversation

DanielSchiavini
Copy link
Collaborator

@DanielSchiavini DanielSchiavini commented Jul 8, 2025

Copy link

vercel bot commented Jul 8, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
curve-dapp ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 16, 2025 1:14pm
curve-dapp-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 16, 2025 1:14pm

@DanielSchiavini DanielSchiavini requested a review from Copilot July 16, 2025 07:36
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces persistence of table filters using local storage and updates related test configurations to align with new import paths and helper functions.

  • Added useTableFilters hook and updated useColumnFilters to use local storage
  • Updated test configs (Vite and TSConfig) with new path aliases and imports
  • Refactored filter and table components (RangeSliderFilter, LlamaMarketsTable) to use the new hooks

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/vite.config.ts Added resolve aliases for UI, kits, and APIs
tests/tsconfig.json Configured path mappings for package imports
tests/cypress/tsconfig.json Extended path mappings for Cypress tests
tests/cypress/support/helpers/lending-mocks.ts Updated API imports and utilization calculation in mocks
tests/cypress/e2e/llamalend/llamalend-markets.cy.ts Wrapped tests in a loop, refactored expandRowOnMobile helper, and updated selectors
packages/tsconfig/cypress.json Switched moduleResolution to bundler
packages/curve-ui-kit/src/shared/ui/DataTable/TableFilters.tsx Imported useTableFilters, removed unused useState, and changed useColumnFilters signature
packages/curve-ui-kit/src/hooks/useLocalStorage.ts Introduced useTableFilters hook
apps/main/src/llamalend/PageLlamaMarkets/filters/RangeSliderFilter.tsx Adjusted range filter logic and hook dependencies
apps/main/src/llamalend/PageLlamaMarkets/LlamaMarketsTable.tsx Added TITLE constant and updated useColumnFilters usage
Comments suppressed due to low confidence (2)

packages/curve-ui-kit/src/shared/ui/DataTable/TableFilters.tsx:144

  • [nitpick] The doc comment for useColumnFilters is outdated after changing the signature to accept tableTitle. Update the comment to explain the purpose of the tableTitle parameter and how filters are persisted.
 */

apps/main/src/llamalend/PageLlamaMarkets/filters/RangeSliderFilter.tsx:60

  • [nitpick] Verify that maxValue matches the intended default maximum value (e.g., defaultValue[1] or a defaultMaximum prop). Inconsistent naming between defaultValue and maxValue can cause confusion or incorrect behavior.
            : [newRange[0] === defaultMinimum ? null : newRange[0], newRange[1] === maxValue ? null : newRange[1]],

Copy link
Collaborator

@0xAlunara 0xAlunara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Table filters are correctly saved in local storage, but the column visibility is not. Might be intended though, not sure. If not, can always be picked up in a different PR.

image

@DanielSchiavini
Copy link
Collaborator Author

Table filters are correctly saved in local storage, but the column visibility is not. Might be intended though, not sure. If not, can always be picked up in a different PR.

Right I didn't include this in the scope, I'll create a separate task

@DanielSchiavini DanielSchiavini merged commit 6303a66 into main Jul 16, 2025
11 checks passed
@DanielSchiavini DanielSchiavini deleted the feat/local-storage branch July 16, 2025 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants